Skip to content

compiler: Preserve compilation invariants under symbol renaming - #3084

Open
FabioLuporini wants to merge 10 commits into
mainfrom
kernel-local-names
Open

FabioLuporini wants to merge 10 commits into
mainfrom
kernel-local-names

Conversation

@FabioLuporini

Copy link
Copy Markdown
Contributor

Generated temporary names can affect CSE scheduling, while symbolic substitutions can reorder lowered arithmetic or leave stale references in auxiliary metadata. Application identifiers such as r0 can also collide with compiler-generated temporaries.

This PR addresses these issues and provides a final IET customization point for backend naming passes:

  • Schedule ready CSE temporaries in expression order, independently of their names.
  • Preserve lowered Add/Mul operand order and grouping during substitution, retaining numeric folding and handling nested negations correctly.
  • Substitute dereference offsets and flattened-array stride metadata while preserving scoped C member names.
  • Distinguish flattened-array macros by their stride bindings.
  • Reserve application and generated identifiers through SymbolRegistry.reserve.
  • Abstract equal symbol instances once, preserving equivalent helper reuse.
  • Add _finalize_iet after dtype lowering and symbol minimization, before macro extraction and signature finalization.

@codecov

codecov Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.30%. Comparing base (a9e3f31) to head (7a1eb98).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3084      +/-   ##
==========================================
+ Coverage   84.27%   84.30%   +0.03%     
==========================================
  Files         258      258              
  Lines       57421    57522     +101     
  Branches     4911     4915       +4     
==========================================
+ Hits        48392    48495     +103     
+ Misses       8205     8204       -1     
+ Partials      824      823       -1     
Flag Coverage Δ
pytest-gpu-aomp-amdgpuX 68.51% <67.74%> (+0.01%) ⬆️
pytest-gpu-gcc- 78.93% <100.00%> (+0.03%) ⬆️
pytest-gpu-icx- 78.86% <100.00%> (+0.05%) ⬆️
pytest-gpu-nvc-nvidiaX 69.21% <67.74%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mloubout mloubout left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks sane

@review-notebook-app

Copy link
Copy Markdown

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

self.reserved.update(i.name for e in exprs for i in e.free_symbols)

# Before indexification, a Function's free symbols omit its own name
self.reserved.update(i.name for i in retrieve_functions(exprs))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mode='unique'

if e0.lhs in e1.rhs.free_symbols:
dag.add_edge(e0, e1, force_add=True)

ordering = {e: n for n, e in enumerate(exprs)}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really better than using .index() on the list?

@_uxreplace.register(CallFromPointer)
def _(expr, rule):
if expr in rule or expr.__class__ in rule:
return _uxreplace.dispatch(object)(expr, rule)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this just do the default _uxreplace? If so, would it be more readable to put that default behaviour in a helper function and call that here too?

Comment thread tests/test_operator.py
x, y = np.indices(f.data_with_halo.shape, dtype=np.float32)
f.data_with_halo[:] = x + 2*y

op = Operator(Eq(u, f.dx + f.dy), name='names_inputs')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to name this

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants